-
Notifications
You must be signed in to change notification settings - Fork 9.4k
magento/magento2#: Replace deprecated methods in back-end controllers of Magento/Customer #24285
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Hi @atwixfirster. Thank you for your contribution
For more details, please, review the Magento Contributor Guide documentation. |
dmytro-ch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@atwixfirster could yo please check the failing unit tests?
ihor-sviziev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @atwixfirster,
Could you review and fix integration tests?
|
@magento run all tests |
done Thanks, guys! |
|
Hi @ihor-sviziev, thank you for the review.
|
|
During testing we faced the issue Problem: "404 Error" message occurs when trying to "Subscribe to Newsletter" via mass action Manual testing scenario:
Actual Result: @atwixfirster Could you take a look, please? Thanks! |
|
Thank you, @engcom-Alfa ! I will check |
|
@dmytro-ch : Should they combine to one?? |
|
@edenduong, these PRs will be combined after approval. |
It's really easier to test each PR separately |
I agree, it depends on PRs. |
|
Hi @ihor-sviziev, thank you for the review. |
sidolov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@atwixfirster please, take a look at the issue reported by @engcom-Alfa
@sidolov, of course . I will check that issue in 3-4 hours. Thanks |
887c9cc to
d1e9194
Compare
fixed Thank you, @engcom-Alfa 👍 |
ihor-sviziev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @atwixfirster,
Unfortunately integration tests started failing after your latest change.
Please review & fix them.
Thank you!
… of Magento/Customer - app/code/Magento/Customer/Controller/Adminhtml/Index/MassAssignGroup.php - app/code/Magento/Customer/Controller/Adminhtml/Index/MassDelete.php - app/code/Magento/Customer/Controller/Adminhtml/Index/MassSubscribe.php - app/code/Magento/Customer/Controller/Adminhtml/Index/ResetPassword.php - app/code/Magento/Customer/Controller/Adminhtml/Index/Save.php - app/code/Magento/Customer/Controller/Adminhtml/Index.php - app/code/Magento/Customer/Controller/Adminhtml/Locks/Unlock.php
d1e9194 to
5842160
Compare
Good morning, @ihor-sviziev! fixed |
|
Hi @dmytro-ch, thank you for the review. |
|
✔️ QA Passed |
…d controllers of Magento/Customer #24285
|
Hi @atwixfirster, thank you for your contribution! |

Description (*)
Pull request replaces deprecated
addError,addSuccess,addExceptionmethodsin the next controllers:
Fixed Issues (if relevant)
Manual testing scenarios (*)
Questions or comments
Contribution checklist (*)
Thank you!